Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replaced 'Middle' with 'Medium' #228

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jjprz
Copy link

@jjprz jjprz commented Nov 30, 2024

πŸ“„ Description

What does this Pull Request do?

  • 🐞 Bug Fix
  • ✨ New Feature
  • πŸ”¨ Code Improvement
  • πŸ“š Documentation Update

✨ Key Changes

  1. Replaced 'Middle' with 'Medium' to ensure that the fan modes appear in the correct order."

πŸ”— Related Issues

Fixes:
Related: esphome/issues#5860


βœ… Checklist

Please ensure the following before submitting your PR:

  • Code compiles without errors πŸ§‘β€πŸ’»
  • All tests pass successfully βœ…
  • Changes have been tested on relevant devices πŸ› οΈ
  • Documentation has been updated (if necessary) πŸ“–
  • This PR is ready for review πŸ”

🌐 Environment Information

πŸ”’ Versions

  • ESPHome Version: 2024.10.4
  • Home Assistant Version: 2024.10.2

🧩 Devices

  • Air Conditioner Type:
    • NASA
    • NON-NASA
    • Other
  • Air Conditioner Model: AC052RNMDKG
  • Outdoor Unit Model: AC052RXADKG
  • ESP Device Model: M5STACK ATOM Lite + M5STACK RS-485
  • Wiring Configuration: F1/F2

πŸ§ͺ Test Plan

How were these changes tested?

  1. It is confirmed that it appears in the correct order
  2. The modes work correctly

@omerfaruk-aran omerfaruk-aran self-requested a review December 5, 2024 08:53
@omerfaruk-aran omerfaruk-aran self-assigned this Dec 5, 2024
@omerfaruk-aran
Copy link
Owner

Thank you for submitting this PR. I've reviewed the changes and have a few questions and observations:

1. Change from Middle to Medium

  • In the current codebase, Middle is used as the fan mode, and it seems to work correctly. Could you clarify if this was not functioning on your side? Did the fan mode labeled as Middle not activate correctly?
  • If this is primarily a naming change (from Middle to Medium), was this update necessary to resolve any functional issues, or is it aimed at standardizing terminology?

2. Compatibility with Existing Systems

  • Considering other users might already be using Middle, this change could potentially break compatibility for them. How do you propose handling this for backward compatibility?
  • Did you test this change with all supported AC models (NASA, Non-NASA, and Other) to ensure consistent functionality?

3. General Observations

  • If this change is purely for naming consistency, it might be better to include both terms (Middle and Medium) for a transition period and document this in the release notes.
  • Please provide additional details about the issue you faced that led to this change. For instance, were there any specific models where Middle failed to function?

Thank you for your work on this. I look forward to your feedback!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants